nit: observeForeverOnce? It makes it clear it isn't bound to any lifecycle though it's a bit confusing, lol.

Yeah, the name isn't very indicative. I could put observeOnceWithoutLifecycleOwner longer name ,but more self explanatory?

nit: this observer name is redundant to the anonymous observer we create. Maybe callback or simple function?

nit: move next to consumeFrom block you're referecing.

Also, it might be helpful to encapsulate them both in a helper function so the grouping is clear:

fun updateSessionControlView() { // maybe a better name
    fun update(state) {
        sessionControlView?.update(state)
    }

    // explain update now
    update(homeFragmentStore.state)
    
    // explain update later
    addObserver(::update)
}

Two best practices in play here:

• when I'm adding code with comments, I always think if it'd be more appropriate to put the code in a function to encapsulate the complexity
• when I'm adding code to lifecycle methods, I always think about using a helper to keep the lifecycle methods short and discussing things at a high level (this doesn't seem to be common on Fenix but I think it greatly helps clarity)

nit: this is unclear. Which consumeFrom block (see my comment below)? What coroutine? What do you mean by "delays our start up"? How? What init block? How does it force our UI to render?

Big issue: what happens if the data isn't available when this is called? We might want to consider blocking on it to make this more deterministic (e.g. if our fnprms starts reporting two drastically different numbers randomly, we'll be confused).

I confirmed this isn't deterministic as is: when running debug builds on Pixel 2, I see the top sites animation.

The list should be empty and will just update itself as its doing now. we could block on it too if needed

nit: I think we need to explain why we do this. We should probably only do so once in the code so maybe say to reference some other method (or the other explanation should reference here?)

We should probably also explain why this is innocuous.

Big issue: I don't love that we're "setting top sites twice" but only dispatching the change once but it seems innocuous on P2. However, on my GS5, the whole homescreen flashes, presumably because it relays out the RV after this line: https://searchfox.org/mozilla-mobile/rev/d81a3c3fce58d3a6c3ebd3257ad1566c9bd462ef/fenix/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt#147 The current behavior does this too but it doesn't look as bad because top sites gets added instead of just replaced.

I'm not sure what we should do about that... Let's discuss

Interesting. I don't see it on my G5. We could do a quick "if this is the first call from the consumeFrom, ignore"

The problem is fixed if we remove that line. The line was added in #5591 and was trying to fix #2874, #5131, and #5375. The first two relate to problems with tabs updating on the homescreen – no longer a problem – and I'm unable to reproduce the 3rd issue (maybe also tabs related)? I tried reproducing similar issues with collections:

• open homescreen, open tabs tray, click 3-dot to add to collection, see homescreen
• open history/bookmarks, add collection, open homescreen

But I can't reproduce. I think this is safe to remove but we should check with ekager.

I think it's confusing that we say, "our info" but really mean "top sites" so I'd rather say "We prefetch our top sites for startup" and it can be changed if the code becomes more general later.

nit: whitespace above

Is this necessary? I don't see anywhere that would cause us to read on the main thread but perhaps I misunderstand.

Not sure why, but our strictmodes complained when I didn't add it and the CI tests failed ( some of them). I could try to remove it

nit: */, not * */

nit: why do we need to draw everything at once? Make it clear this is a perf issue

nit: I like the inner function version I suggested because it makes it clear that the code is repeated and prevents someone from missing that if they decide to refactor this. But I can go either way

nit: In the extended commit summary, I would have included information on why it's okay to remove this code (though I guess it's fine if it's just in the PR text? maybe you can link to what's in the PR).